Refactor storage content-type handling of SignedURL by ChristopherHX · Pull Request #36804 · go-gitea/gitea

ChristopherHX · 2026-03-02T11:52:02Z

replace raw url.Values by *storage.SignedURLParam
implement content-type in azblob
implement content-disposition in azblob
add tests for content types in response

Needed for actions artifact backend to control the headers: Feature non zipped actions artifacts (action v7 / nodejs/npm v6.2.0) #36786

Add checks to avoid setting empty response parameters for Minio.

ChristopherHX · 2026-03-02T13:13:59Z

modules/public/mime_types.go

+	"image/gif",
+	"image/webp",
+	"image/avif",
+	// ATTENTION! Don't support unsafe types like HTML/SVG due to security concerns: they can contain JS code, and maybe they need proper Content-Security-Policy


Does not the blob storage already has some sort of domain sandbox???

If served directly from giteas domain, I would agree. Given you can not make assumptions about the blob signed url you cannot really reference anything more than ca. 5min that is part of the same domain.

e.g. this is how GitHub permits raw html artifacts to be served without sandbox

ChristopherHX · 2026-03-02T13:16:20Z

modules/storage/azureblob.go

 	return convertAzureBlobErr(err)
 }

+func (a *AzureBlobStorage) GetSasURL(b *blob.Client, template sas.BlobSignatureValues) (string, error) {


the function from the azure package does not allow content-type + disposition to be provided. Since it is part of the signature we cannot just append this to the url.

Copilot

Pull request overview

This PR refactors the storage content-type handling for signed/presigned object storage URLs. Instead of passing raw url.Values for request parameters to ObjectStorage.URL(), it introduces a strongly-typed SignedURLParam struct with ContentType and ContentDisposition fields. It also implements proper content-type and content-disposition header support in Azure Blob Storage (which was previously missing, marked as a TODO), and adds integration tests for both Minio and Azure Blob Storage backends.

Changes:

Introduces SignedURLParam struct with a WithDefaults(name) method that auto-detects safe MIME types from file extensions (replacing duplicated inline MIME type maps).
Updates all ObjectStorage.URL() implementations (MinioStorage, AzureBlobStorage, LocalStorage, discardStorage) and all callers to use *SignedURLParam instead of url.Values.
Adds Azure Blob Storage SAS URL signing with ContentType and ContentDisposition support via a new GetSasURL helper, and adds CI integration tests for both Minio and Azure backends.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`modules/storage/storage.go`	Adds `SignedURLParam` struct and `WithDefaults` method; updates `ObjectStorage` interface signature
`modules/storage/minio.go`	Refactors `URL` to use `*SignedURLParam` and `WithDefaults` instead of inline MIME type map
`modules/storage/azureblob.go`	Adds `GetSasURL` helper and updates `URL` to include ContentType/ContentDisposition in SAS signing
`modules/storage/local.go`	Updates `URL` signature to match new interface
`modules/storage/helper.go`	Updates `discardStorage.URL` signature to match new interface
`modules/public/mime_types.go`	Adds `wellKnownSafeMimeTypes` list and two new exported functions
`modules/packages/content_store.go`	Updates `GetServeDirectURL` signature to use `*storage.SignedURLParam`
`services/packages/packages.go`	Updates `OpenBlobForDownload` signature to use `*storage.SignedURLParam`
`routers/api/packages/container/container.go`	Uses `&storage.SignedURLParam{ContentType: ...}` instead of `url.Values` for container blob serving
`modules/storage/storage_test.go`	Adds shared test helper for content-type/disposition URL response validation
`modules/storage/minio_test.go`	Adds CI integration test for Minio content-type/disposition URL behavior
`modules/storage/azureblob_test.go`	Adds CI integration test for Azure Blob content-type/disposition URL behavior

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

modules/storage/storage.go

modules/storage/azureblob.go

modules/storage/storage.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: ChristopherHX <christopher.homberger@web.de>

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-02T19:48:15Z

modules/storage/azureblob.go

+	t, err := time.Parse(blob.SnapshotTimeFormat, urlParts.Snapshot)
+	if err != nil {
+		t = time.Time{}


time.Parse will return a non-nil error when urlParts.Snapshot is an empty string (i.e., when the blob has no snapshot), and the error is silently swallowed. While defaulting to time.Time{} (zero time) is likely correct for a non-snapshot blob, the unconditional error suppression hides genuine parse failures. Consider only setting t = time.Time{} when urlParts.Snapshot is empty, and propagating the error otherwise.

Suggested change

t, err := time.Parse(blob.SnapshotTimeFormat, urlParts.Snapshot)

if err != nil {

t = time.Time{}

var t time.Time

if urlParts.Snapshot == "" {

t = time.Time{}

} else {

t, err = time.Parse(blob.SnapshotTimeFormat, urlParts.Snapshot)

if err != nil {

return "", err

}

Copilot · 2026-03-02T19:48:15Z

modules/storage/azureblob_test.go

+			Endpoint: "http://devstoreaccount1.azurite.local:10000",
+			// https://learn.microsoft.com/azure/storage/common/storage-use-azurite?tabs=visual-studio-code#well-known-storage-account-and-key
+			AccountName: "devstoreaccount1",
+			AccountKey:  "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==",


This is the well-known Azurite development account key, which is safe for local testing. However, hardcoding it directly in source may trigger secret scanning tools. Consider referencing it from an environment variable or a named constant with a comment clarifying it is the public Azurite development key, consistent with how similar credentials are typically handled.

Copilot · 2026-03-02T19:48:16Z

modules/storage/storage_test.go

+	testSingleBlobStorageURLContentTypeAndDisposition(t, s, "test.txt", "test.pdf", SignedURLParam{
+		ContentType:        "application/pdf",
+		ContentDisposition: "inline",
+	}, nil)


The file saved is test.txt (plain text content), but the URL is requested with the name "test.pdf". The withDefaults logic uses the name parameter to detect MIME type and disposition, so the test is asserting that an object with a .pdf name gets ContentType: "application/pdf" and ContentDisposition: "inline" — but the expected value here reflects the request parameters inferred from the name, not the actual server response content-type of the stored object. If the storage backend serves the stored object's own content-type (e.g., Minio may sniff it), the assertion may fail or be misleading. Ensure the test accurately reflects what the server will actually return in the Content-Type header.

Copilot · 2026-03-02T19:48:16Z

modules/storage/storage_test.go

+	s, err := NewStorage(typStr, cfg)
+	assert.NoError(t, err)
+
+	data := "Q2xTckt6Y1hDOWh0"


The test data string "Q2xTckt6Y1hDOWh0" is an unexplained magic value. Adding a comment clarifying what this string represents (e.g., arbitrary test data, a base64-encoded value) would improve readability.

Suggested change

data := "Q2xTckt6Y1hDOWh0"

data := "Q2xTckt6Y1hDOWh0" // arbitrary test content; specific value is irrelevant to this test

Signed-off-by: ChristopherHX <christopher.homberger@web.de>

Refactor storage content-type handling of SignedURL

a583f46

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 2, 2026

github-actions bot added modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code labels Mar 2, 2026

.

6e87c19

ChristopherHX mentioned this pull request Mar 2, 2026

Feature non zipped actions artifacts (action v7 / nodejs/npm v6.2.0) #36786

Draft

8 tasks

ChristopherHX added 2 commits March 2, 2026 13:05

close body

74a0020

Avoid setting empty response parameters in Minio URL

f32f9fb

Add checks to avoid setting empty response parameters for Minio.

ChristopherHX commented Mar 2, 2026

View reviewed changes

ChristopherHX added type/refactoring Existing code has been cleaned up. There should be no new functionality. type/enhancement An improvement of existing functionality labels Mar 2, 2026

silverwind requested a review from Copilot March 2, 2026 14:36

Copilot started reviewing on behalf of silverwind March 2, 2026 14:37 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

modules/storage/storage.go Outdated Show resolved Hide resolved

modules/storage/azureblob.go Outdated Show resolved Hide resolved

modules/storage/storage.go Outdated Show resolved Hide resolved

modules/storage/storage.go Outdated Show resolved Hide resolved

ChristopherHX and others added 2 commits March 2, 2026 16:10

Apply suggestions from code review

7481389

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: ChristopherHX <christopher.homberger@web.de>

apply suggestions

a9efd95

ChristopherHX requested a review from Copilot March 2, 2026 19:43

Copilot AI reviewed Mar 2, 2026

View reviewed changes

Copilot started reviewing on behalf of ChristopherHX March 2, 2026 19:58 View session

ChristopherHX mentioned this pull request Mar 7, 2026

Gitea Actions is incompatible with actions/upload-artifact@v6 and later versions #36829

Open

Merge branch 'main' into refac-blob-storage-mime-types

dda9fca

Signed-off-by: ChristopherHX <christopher.homberger@web.de>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor storage content-type handling of SignedURL#36804

Refactor storage content-type handling of SignedURL#36804
ChristopherHX wants to merge 7 commits intogo-gitea:mainfrom
ChristopherHX:refac-blob-storage-mime-types

ChristopherHX commented Mar 2, 2026 •

edited

Loading

Uh oh!

ChristopherHX Mar 2, 2026

Uh oh!

ChristopherHX Mar 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	data := "Q2xTckt6Y1hDOWh0"
	data := "Q2xTckt6Y1hDOWh0" // arbitrary test content; specific value is irrelevant to this test

Uh oh!

Conversation

ChristopherHX commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChristopherHX Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

ChristopherHX Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ChristopherHX commented Mar 2, 2026 •

edited

Loading